Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Code Health] Clang Tidy cleanup, Part 2 #3038

Merged
merged 11 commits into from
Aug 25, 2024
Merged

[Code Health] Clang Tidy cleanup, Part 2 #3038

merged 11 commits into from
Aug 25, 2024

Conversation

msiddhu
Copy link
Contributor

@msiddhu msiddhu commented Aug 20, 2024 •
edited by marcalff
Loading

Contributes to #2053

Changes

In continuation to Cleanup 1, this changes reduces a lot of clang-tidy warnings and removed some checks that are not relevant.

For significant contributions please make sure you have completed the following items:

  • CHANGELOG.md updated for non-trivial changes
  • Unit tests have been added
  • Changes in public API reviewed

@codecov Codecov
Copy link

codecov bot commented Aug 21, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 47.05882% with 9 lines in your changes missing coverage. Please review.

Project coverage is 87.63%. Comparing base (497eaf4) to head (0e144cc).
Report is 120 commits behind head on main.

Files Patch % Lines
...k/src/metrics/aggregation/lastvalue_aggregation.cc 40.00% 6 Missing ⚠️
sdk/src/metrics/aggregation/sum_aggregation.cc 0.00% 2 Missing ⚠️
sdk/src/logs/logger_provider.cc 50.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3038      +/-   ##
==========================================
+ Coverage   87.12%   87.63%   +0.52%     
==========================================
  Files         200      190      -10     
  Lines        6109     5868     -241     
==========================================
- Hits         5322     5142     -180     
+ Misses        787      726      -61
Files Coverage Δ
...include/opentelemetry/sdk/common/circular_buffer.h 100.00% <ø> (ø)
sdk/src/common/base64.cc 82.93% <100.00%> (+0.53%) ⬆️
sdk/src/metrics/state/sync_metric_storage.cc 100.00% <100.00%> (ø)
sdk/src/trace/batch_span_processor.cc 94.12% <100.00%> (+1.70%) ⬆️
sdk/src/logs/logger_provider.cc 90.91% <50.00%> (ø)
sdk/src/metrics/aggregation/sum_aggregation.cc 75.81% <0.00%> (ø)
...k/src/metrics/aggregation/lastvalue_aggregation.cc 65.63% <40.00%> (-1.04%) ⬇️

... and 116 files with indirect coverage changes

@marcalff
Copy link
Member

@msiddhu

This set of changes looks good already. Please tag as ready for review, so this part can be merged.

@msiddhu msiddhu marked this pull request as ready for review August 25, 2024 16:28
@msiddhu msiddhu requested a review from a team August 25, 2024 16:28
marcalff
marcalff approved these changes Aug 25, 2024
View reviewed changes
Copy link
Member

@marcalff marcalff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the cleanup.

@marcalff marcalff merged commit a71642f into open-telemetry:main Aug 25, 2024
52 checks passed
malkia added a commit to malkia/opentelemetry-cpp that referenced this pull request Aug 25, 2024
@malkia
Merge pull request #63 from open-telemetry/main
3298107 
[Code Health] Clang Tidy cleanup, Part 2 (open-telemetry#3038)
@BrewTestBot BrewTestBot mentioned this pull request Oct 8, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marcalff marcalff marcalff approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@msiddhu @marcalff